project-clone/internal/global.go (1)

68-76: Consider adding an upper bound for CloneRetries.

The validation rejects negative values but allows arbitrarily large retry counts. With exponential backoff, high values like CloneRetries=30 would result in a delay of ~17 years before the final retry attempt (1s * 2^29).

Consider capping at a reasonable maximum (e.g., 10) to prevent configuration mistakes from causing excessively long waits.

Suggested change

+const maxCloneRetries = 10
+
 CloneRetries = defaultCloneRetries
 if val := os.Getenv(cloneRetriesEnvVar); val != "" {
     parsed, err := strconv.Atoi(val)
-    if err != nil || parsed < 0 {
-        log.Printf("Invalid value for %s: %q, using default (%d)", cloneRetriesEnvVar, val, defaultCloneRetries)
+    if err != nil || parsed < 0 || parsed > maxCloneRetries {
+        log.Printf("Invalid value for %s: %q (must be 0-%d), using default (%d)", cloneRetriesEnvVar, val, maxCloneRetries, defaultCloneRetries)
     } else {
         CloneRetries = parsed
     }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@project-clone/internal/global.go` around lines 68 - 76, The env parsing for
CloneRetries (using cloneRetriesEnvVar and defaultCloneRetries) only rejects
negatives but allows huge values; add an upper bound (e.g., maxCloneRetries =
10) and when parsed > maxCloneRetries log a warning and set CloneRetries =
maxCloneRetries, otherwise set CloneRetries = parsed; keep the existing
invalid-number fallback behavior and use the same log.Printf pattern to report
out-of-range values.

project-clone/internal/git/setup.go (1)

66-68: Use %w for proper error wrapping.

Using %s formats the error as a string but loses the error chain, preventing callers from using errors.Is() or errors.Unwrap() to inspect the underlying cause.

Suggested change

 if cloneErr != nil {
-    return fmt.Errorf("failed to clone project: %s", cloneErr)
+    return fmt.Errorf("failed to clone project: %w", cloneErr)
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@project-clone/internal/git/setup.go` around lines 66 - 68, The error return
currently formats cloneErr with "%s" which discards the error chain; update the
return in the clone error path to wrap the original error using "%w" so callers
can use errors.Is/errors.Unwrap (i.e., change the return that references
cloneErr to use "%w" to wrap the error).

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

project-clone/internal/global.go (2)

38-40: Use guideline-compliant constant naming for new retry constants.

cloneRetriesEnvVar, defaultCloneRetries, and maxCloneRetries are lower camelCase. Rename them to PascalCase or UPPER_SNAKE_CASE to align with the repo’s Go constant naming rule.

As per coding guidelines: "Use PascalCase or UPPER_SNAKE_CASE for constant names."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@project-clone/internal/global.go` around lines 38 - 40, The constants
cloneRetriesEnvVar, defaultCloneRetries, and maxCloneRetries use lowerCamelCase;
rename them to follow the project's constant naming rule (PascalCase or
UPPER_SNAKE_CASE) throughout the file and all usages—e.g.,
ProjectCloneRetriesEnvVar or CLONE_RETRIES_ENV_VAR, DefaultCloneRetries or
DEFAULT_CLONE_RETRIES, and MaxCloneRetries or MAX_CLONE_RETRIES—update their
declarations and all references (search for cloneRetriesEnvVar,
defaultCloneRetries, maxCloneRetries) to the new names to ensure compilation and
consistency.

---

18-31: Reorder import groups to match repository Go conventions.

The import block currently places a project-local package before third-party imports. Please order as: standard library, third-party/Kubernetes, then project-local.

As per coding guidelines: "Organize imports into three groups separated by blank lines: (1) standard library, (2) third-party + Kubernetes, (3) project-local."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@project-clone/internal/global.go` around lines 18 - 31, The import block is
misordered: move imports into three groups (standard library,
third-party/Kubernetes, then project-local) separated by blank lines;
specifically keep "errors", "fmt", "log", "os", "regexp", "strconv", "time"
together as the first group, then place go-git imports ("github.com/go-git/...",
"github.com/kevinburke/ssh_config") and any other third-party/Kubernetes
packages as the second group, and place the project-local package
"github.com/devfile/devworkspace-operator/pkg/library/constants" as the final
group; update the import ordering in the file's import block accordingly.